Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(plugin-keychain-memory): add REST API endpoint implementations #2893

Conversation

petermetz
Copy link
Contributor

@petermetz petermetz commented Nov 16, 2023

BREAKING CHANGE: Some of the schema elements that were redundant
got removed from the openapi.json spec of the memory plugin.
The same schemas are now being pulled from the core-api package.
In this sense there is no breaking change because these two
schemas were already equivalent anyway, but the fact is
the openapi.json had schema elements removed from it anyway
so this is something that people need to be aware of as they
upgrade to v2.0.0 in the near future.

  1. Added the REST API endpoints to the plugin so that it is accessible over
    HTTP for testing/demo purposes as necessary.
  2. Also refactored the openapi.json spec file so that it pulls the schema
    definitions from the core-api package the same way as the
    keychain-memory-wasm plugin is doing already.
  3. Migrated the test API client test case to Jest from tape
  4. Refactored the handleRequest methods of the endpoint classes so that
    they perform the error handling with respect to the HTTP status codes being
    set correctly (sanitization is pending because I have to send a separate
    PR for that to the cactus-core package)

Special thanks to @outSH for the suggestions to improve things!

Signed-off-by: Peter Somogyvari [email protected]


Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

Copy link
Contributor

@outSH outSH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments

**BREAKING CHANGE**: Some of the schema elements that were redundant
got removed from the openapi.json spec of the memory plugin.
The same schemas are now being pulled from the core-api package.
In this sense there is no breaking change because these two
schemas were already equivalent anyway, but the fact is
the openapi.json had schema elements removed from it anyway
so this is something that people need to be aware of as they
upgrade to v2.0.0 in the near future.

1. Added the REST API endpoints to the plugin so that it is accessible over
HTTP for testing/demo purposes as necessary.
2. Also refactored the openapi.json spec file so that it pulls the schema
definitions from the core-api package the same way as the
keychain-memory-wasm plugin is doing already.
3. Migrated the test API client test case to Jest from tape
4. Refactored the `handleRequest` methods of the endpoint classes so that
they perform the error handling with respect to the HTTP status codes being
set correctly (sanitization is pending because I have to send a separate
PR for that to the cactus-core package)

Special thanks to @outSH for the suggestions to improve things!

Signed-off-by: Peter Somogyvari <[email protected]>
@petermetz petermetz force-pushed the feat-keychain-memory-rest-api-endpoints branch from 8d0b9ac to 48f2209 Compare December 5, 2023 22:15
@petermetz petermetz requested a review from outSH December 5, 2023 22:24
petermetz referenced this pull request in petermetz/cacti Dec 5, 2023
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.
2. The yarn.lock file seems to have gotten out of date by accident again
so I'm also sneaking in that as an update here just to get the fix in
ASAP and without burning too much on CI execution costs.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz referenced this pull request in petermetz/cacti Dec 6, 2023
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.
2. The yarn.lock file seems to have gotten out of date by accident again
so I'm also sneaking in that as an update here just to get the fix in
ASAP and without burning too much on CI execution costs.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
(cherry picked from commit 7cf4a73)
@petermetz petermetz merged commit c7a8fa5 into hyperledger-cacti:main Dec 6, 2023
106 of 136 checks passed
petermetz referenced this pull request in petermetz/cacti Dec 7, 2023
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.
2. The yarn.lock file seems to have gotten out of date by accident again
so I'm also sneaking in that as an update here just to get the fix in
ASAP and without burning too much on CI execution costs.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
(cherry picked from commit 7cf4a73)
@petermetz petermetz deleted the feat-keychain-memory-rest-api-endpoints branch December 8, 2023 03:18
petermetz referenced this pull request in petermetz/cacti Dec 15, 2023
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.
2. The yarn.lock file seems to have gotten out of date by accident again
so I'm also sneaking in that as an update here just to get the fix in
ASAP and without burning too much on CI execution costs.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
(cherry picked from commit 7cf4a73)
petermetz referenced this pull request in petermetz/cacti Dec 21, 2023
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz referenced this pull request in petermetz/cacti Jan 4, 2024
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz referenced this pull request in petermetz/cacti Jan 9, 2024
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.
2. The yarn.lock file seems to have gotten out of date by accident again
so I'm also sneaking in that as an update here just to get the fix in
ASAP and without burning too much on CI execution costs.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
(cherry picked from commit 7cf4a73)
petermetz referenced this pull request in petermetz/cacti Jan 17, 2024
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz referenced this pull request Jan 17, 2024
1. This is a security fix so that the exception serialization does not
accidentally XSS anybody who is looking at crash logs through some
admin GUI that is designed to show logs that are considered trusted.

Related discussion about `1)` can be seen at this other pull request:
https://github.com/hyperledger/cacti/pull/2893

Signed-off-by: Peter Somogyvari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants